Skip to content

[Core] Explicitly Support gzip & deflate for Encodings in Aiohttp - #48388

Open
kashifkhan wants to merge 9 commits into
Azure:mainfrom
kashifkhan:azcore_comp_hdr
Open

[Core] Explicitly Support gzip & deflate for Encodings in Aiohttp#48388
kashifkhan wants to merge 9 commits into
Azure:mainfrom
kashifkhan:azcore_comp_hdr

Conversation

@kashifkhan

Copy link
Copy Markdown
Member

This PR addresses an issue with aiohttp and core. Today aiohttp automatically appends  br  (Brotli) and  zstd  to the Accept-Encoding  request header when it detects the corresponding optional library (e.g.  brotli / brotlicffi ) installed in the environment.

When the service honors that and returns a Brotli-encoded response, azure-core can't decode it: the aiohttp transport runs with auto_decompress=False  and decompresses manually, supporting only  gzip / deflate , so the raw bytes reach response.text() and raise a UnicodeDecodeError

Request does similar, but lets requests/urllib3 decompress the response itself, so it decodes whatever it advertised

We do the following:

  • Explicitly pass in gzip and deflate for aiohttp
  • Move the aiohttp body helper into the aiohttp section; its not generic and used only by aiohttp
  • Update tests

Copilot AI review requested due to automatic review settings July 31, 2026 14:56
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
7 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Restricts AioHttpTransport to supported response encodings, preventing decoding failures when optional aiohttp codecs are installed.

Changes:

  • Defaults Accept-Encoding to gzip, deflate while preserving caller overrides.
  • Relocates the aiohttp-specific body helper.
  • Adds tests and updates package metadata.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
azure/core/pipeline/transport/_aiohttp.py Sets supported encodings and hosts the body helper.
azure/core/rest/_aiohttp.py Imports the relocated helper.
azure/core/utils/_pipeline_transport_rest_shared.py Removes aiohttp-specific logic.
tests/async_tests/test_universal_http_async.py Tests defaults and caller overrides.
CHANGELOG.md Documents the fix.
azure/core/_version.py Bumps the version to 1.42.0.

Comment thread sdk/core/azure-core/CHANGELOG.md Outdated
Copilot AI review requested due to automatic review settings July 31, 2026 15:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Comment thread sdk/core/azure-core/azure/core/pipeline/transport/_aiohttp.py

@JennyPng JennyPng left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me

@github-actions

This comment has been minimized.

Copilot AI review requested due to automatic review settings July 31, 2026 20:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@github-actions

This comment has been minimized.

Copilot AI review requested due to automatic review settings August 3, 2026 14:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

sdk/core/azure-core/azure/core/pipeline/transport/_aiohttp.py:345

  • A preconfigured ClientSession can supply its own default Accept-Encoding (for example, identity or an encoding supported by its custom decompressor). aiohttp gives these per-request headers precedence over session defaults, so this unconditional request default silently replaces that user configuration. Check the session headers before adding the transport fallback, and add coverage for an injected session with a custom default header.
        request.headers.setdefault("Accept-Encoding", _SUPPORTED_ACCEPT_ENCODING)

Copilot AI review requested due to automatic review settings August 3, 2026 15:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

sdk/core/azure-core/azure/core/pipeline/transport/_aiohttp.py:191

  • This only constrains sessions created by the transport. An injected aiohttp.ClientSession(auto_decompress=False) with no explicit Accept-Encoding still falls back to ClientRequest.DEFAULT_HEADERS, so environments with Brotli/Zstandard installed can continue advertising br/zstd; send() then selects azure-core's gzip/deflate-only manual decompression and the original decode failure remains. When auto_decompress is false, set the supported header at request time only if neither the request nor the injected session supplies one, and add coverage for that injected-session case.
                    "headers": {"Accept-Encoding": _SUPPORTED_ACCEPT_ENCODING},

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor
[Pilot] PR Pipeline Failure Analysis

A CI pipeline failed on this pull request. Here is an automated analysis of what went wrong and how to get the build green.

What failed

Three distinct failure groups were detected:

  1. [Build/Infrastructure] pnpm dependency resolution — Pipeline 6651431 failed during a build task with ERR_PNPM_BAD_CONFIG_DEP: Cannot resolve pnpm@9.5.0 as a package manager dependency because it has no integrity. This is a tooling/infrastructure issue unrelated to PR code changes.

  2. [Test] azure-ai-textanalytics — 14 tests in tests.test_* (e.g., test_abstract_summary, test_recognize_entities, test_text_sentiment, etc.) failed consistently across macOS (Python 3.11), Ubuntu (Python 3.10), and Windows (Python 3.12) — all three platforms and all install modes (whl, sdist, mindependency). The breadth of failures across platforms and test categories suggests a service-side or environment issue rather than a regression from this PR.

  3. [Test] azure-cosmos retry policiesTestServiceRetryPolicies.test_service_request_retry_policy (Python 3.10) and test_service_response_retry_policy (Python 3.11) failed on Windows Emulator tests. These are unrelated to the aiohttp Accept-Encoding changes in this PR.

Recommended next steps

  • For the pnpm failure: Re-run the pipeline — this is an infrastructure/dependency resolution issue (pnpm@9.5.0 integrity missing). No code changes should be required. If it recurs, it may need to be escalated to the engineering system team.
  • For the textanalytics and cosmos failures: Check whether these failures are pre-existing or appear on main as well. They span many unrelated test areas and all platforms, pointing to environment/service issues rather than a regression introduced by this PR.
  • See the CI troubleshooting guide: https://aka.ms/ci-fix
  • Push new commits to address any failures confirmed to be caused by this PR; this comment updates automatically on the next failing run.
Raw pipeline analysis (azsdk ci analyze)
Analyzing pipeline https://github.com/Azure/azure-sdk-for-python/pull/48388...
--------------------------------------------------------------------------------
Failed Tests
--------------------------------------------------------------------------------
Cosmos (Windows/Python3.10): tests.test_service_retry_policies.TestServiceRetryPolicies.test_service_request_retry_policy
Cosmos (Windows/Python3.11): tests.test_service_retry_policies.TestServiceRetryPolicies.test_service_response_retry_policy

azure-ai-textanalytics (macOS/Ubuntu/Windows, mindependency+whl+sdist):
  test_abstract_summary, test_extract_key_phrases, test_extract_summary,
  test_health_care_lro, test_language_detection, test_multi_label_classify,
  test_recognize_entities, test_recognize_entities_ner,
  test_recognize_linked_entities, test_recognize_pii,
  test_recognize_pii_confidence_score, test_recognize_pii_redaction_policies,
  test_single_label_classify, test_text_sentiment

--------------------------------------------------------------------------------
Failed Tasks
--------------------------------------------------------------------------------
### Errors:
ERR_PNPM_BAD_CONFIG_DEP
  × resolve package manager dependencies
  ╰─▶ Cannot resolve pnpm@9.5.0 as a package manager dependency because it has no integrity

### Pipeline: https://dev.azure.com/azure-sdk/public/_build/results?buildId=6651431

Copilot detected the failing pipeline and generated the analysis above. To have it attempt a fix automatically, reply with @copilot please fix the failing pipeline on this PR.

Generated by Pipeline Analysis - Next Steps · 28.6 AIC · ⌖ 6.32 AIC · ⊞ 6.6K ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants